feat(help): Add styling to help output #12578

epage · 2023-08-28T17:12:00Z

What does this PR try to resolve?

Try to make --help output easier to parse by using terminal styling

Screenshots:

(nargo is my shell script wrapping cargo run --manifest-path cargo/Cargo.toml)

How should we test and review this PR?

At this time, the only styling snapshotting library I know of is a pain to use, so testing this requires manually running the commands which I did. Screenshots are included for easier evaluation of the general idea.

Snapshotting of the plain text output ensures we don't have accidental formatting regressions from this change since the formatting isn't as obvious from looking at the code.

Additional information

Traditionally, cargo has disabled clap's styled output. My assumed
reason is that cargo mixes custom help output with auto-generated and
you couldn't previously make it all styled.
Clap 4.2 allowed users to pass in strings styled using ANSI escape
codes, allowing us to pass in styled text that matches clap, unblocking this. In clap
4.4.1, clap gained the ability for the user to override the style.

In this PR, I decided to use the new 4.4.1 feature to style clap's
output to match the rest of cargo's output. Alternatively, we could use
a more subdue style that clap uses by default.

I used the color-print crate to allow something almost html-like for styling &static str. Alternatively, we could directly embed the ANSI escape codes harder to get write, harder to inspect), or we could do the styling at runtime and enable the string feature in clap.

I decided to not style Arg::help messages because

It might be distracting to have the descriptions lit up like a
christmas tree
It is a lot more work

The one exception I made was for --list since it is for a
psuedo-command (...) and I wanted to intentionally draw attention to
it.

#12593 made styling of cargo -h cleaner imo.
#12592 and #12594 were improvements I noticed while doing this.

rustbot · 2023-08-28T17:12:05Z

r? @weihanglo

(rustbot has picked a reviewer for you, use r? to override)

epage · 2023-08-28T17:18:42Z

Before applying this to the rest of cargo's commands, I figured I'd post this draft for feedback from the cargo team on (1) if we want this and (2) if we should make any tweaks

In working on rust-lang#12578, I'm focusing on each help string to decide how it should be handled and I noticed this. It feels weird to explain something in terms of another command's CLI, so I took `rustc --help`s message and added `rustc` to clarify it. In reflecting on this, I'm not 100% convinced and am open to other opinions.

In working on rust-lang#12578, I'm focusing on each help string to decide how it should be handled and I noticed this. It feels weird to explain something in terms of another command's CLI, so I took `rustc --help`s message and added `rustc` to clarify it. Looking back, the flag was added in rust-lang#2551 with the message we have today. Nothing seems to really be said about it. In reflecting on this, I'm not 100% convinced and am open to other opinions.

In working on rust-lang#12578, I felt it would be weird to style the entire statement about commands but it also felt weird to not style it. So this change explores and alternatively way of communicating the information.

In working on rust-lang#12578, I felt it would be weird to style the entire statement about commands but it also felt weird to not style it. So this change explores an alternatively way of communicating the information.

fix(help): Explain --explain In working on #12578, I'm focusing on each help string to decide how it should be handled and I noticed this. It feels weird to explain something in terms of another command's CLI, so I took `rustc --help`s message and added `rustc` to clarify it. Looking back, the flag was added in #2551 with the message we have today. Nothing seems to really be said about it. In reflecting on this, I'm not 100% convinced and am open to other opinions.

Auditing all of the `--help` in prep for rust-lang#12578 and noticed that we list the VCS information twice, once on our end and once by clap.

fix(help): Remove redundant information from new/init Auditing all of the `--help` in prep for #12578 and noticed that we list the VCS information twice, once on our end and once by clap.

In working on rust-lang#12578, I'm focusing on each help string to decide how it should be handled and I noticed this. It feels weird to explain something in terms of another command's CLI, so I took `rustc --help`s message and added `rustc` to clarify it. Looking back, the flag was added in rust-lang#2551 with the message we have today. Nothing seems to really be said about it. In reflecting on this, I'm not 100% convinced and am open to other opinions.

fix(help): Explain --explain In working on #12578, I'm focusing on each help string to decide how it should be handled and I noticed this. It feels weird to explain something in terms of another command's CLI, so I took `rustc --help`s message and added `rustc` to clarify it. Looking back, the flag was added in #2551 with the message we have today. Nothing seems to really be said about it. In reflecting on this, I'm not 100% convinced and am open to other opinions.

In working on rust-lang#12578, I felt it would be weird to style the entire statement about commands but it also felt weird to not style it. So this change explores an alternatively way of communicating the information.

fix(help): Provide better commands heading for styling In working on #12578, I felt it would be weird to style the entire statement about commands but it also felt weird to not style it. So this change explores an alternatively way of communicating the information.

bors · 2023-08-30T20:37:21Z

☔ The latest upstream changes (presumably #12593) made this pull request unmergeable. Please resolve the merge conflicts.

This mirrors what the API in rust-lang/cargo#12655. I'm hoping to have the `anstyle` consts be a public crate owned by the cargo team in the future after which we'll refactor to re-export it. See rust-lang/cargo#12578 for adding the style.

refactor: Consolidate clap/shell styles ### What does this PR try to resolve? This is a follow up to #12578 to reduce duplication of terminal styling. This still leaves styling in `color_print::cstr!`. This is also prep for copy/pasting into `clap-cargo` for others to use. Another step might be to extract `cargo::util::style` into its own crate. ### How should we test and review this PR? We have no styling tests so this can only be verified by inspection or running the commands ### Additional information I chose `anstyle` for expressing these as its an API designed specifically for stable style definitions to put in public APIs.

Update cargo 11 commits in 2fc85d15a542bfb610aff7682073412cf635352f..d5336f813df39d476e61fc46daabb1446350660a 2023-09-09 01:49:46 +0000 to 2023-09-14 19:55:49 +0000 - fix: emit a warning for `credential-alias` shadowing (rust-lang/cargo#12671) - refactor: fix lint errors in preparation of `[lints]` table integration (rust-lang/cargo#12669) - Limit cargo add feature print (rust-lang/cargo#12662) - Clippy (rust-lang/cargo#12667) - Prerelease candidates error message (rust-lang/cargo#12659) - Fix typos: `informations` -> `information` (rust-lang/cargo#12666) - chore: update globset to 0.4.13 (rust-lang/cargo#12665) - refactor: Consolidate clap/shell styles (rust-lang/cargo#12655) - libgit2 fixed upstream (rust-lang/cargo#12657) - Index summary enum (rust-lang/cargo#12643) - feat(help): Add styling to help output (rust-lang/cargo#12578) r? ghost

Add colored help to be consistent with Cargo On rust-lang/cargo#12578, a new colored help message format was introduced. This PR introduces the same styling from that `cargo help` message to our `cargo clippy --help` message. More information is provided in the original PR, fixes #11482. The perfect reviewing process would be that the reviewer installs this branch and checks for themselves, but here are some screenshots, there are some more screenshots in the original issue. ![image](https://github.com/rust-lang/rust-clippy/assets/73757586/0c4d1b6d-5aa2-4146-a401-9ae88f6dedf5) (Note that the actual text may change in the actual commit, that screenshot is just to test the colors). Also note that the `color-print` version **should always** be synced with Cargo's `color-print` version to avoid increasing build times in the rust-lang/rust repo. changelog:Add colors to the `cargo clippy --help` output 🎉.

your-diary · 2023-11-18T07:50:31Z

How can I disable this or change the used colors?

--color never didn't work: cargo --color never build --help

epage · 2023-11-18T12:11:46Z

Could you go into why you want to disable it? It matches existing cargo colors so we thought it'd be innocuous. It'd be good to know what problems people are having with it.

As for disabling, the cli doesn't work because help gets processed before we can process --color. I can't remember if I hacked in term.color / CARGO_TERM_COLOR support but that'd be the next thing id recommend. Otherwise, as an implementation detail, we do support NO_COLOR=1 (I think there is an underscore in there).

your-diary · 2023-11-18T14:05:50Z

@epage

Could you go into why you want to disable it?

Because personally I don't like the colors used.
Additionally I believe there are not small number of people who want to opt-out the colorful output entirely, taking into account how many people in the world use Rust.
How colors look to one's eyes highly depends on the person.
As personal preferences of course significantly vary from person to person, I expect the command lets users customize its appearance as much as possible.
I believe this is one of the reasons why many CUI commands have --color={auto|never|always} option.
Some commands like ls even lets users customize which colorset is used (i.e. LS_COLORS).

It matches existing cargo colors

I don't think so, partly. The first impression for the current output I had was "too bright".

Actually, as you say, the color cyan has been used in Cargo. But, as far as I know, the main color is green and cyan is just temporarily displayed for highlighting purposes:

However, in the output of --help, cyan is used as one of the main colors:

This looks too bright for me.

As for disabling, the cli doesn't work because help gets processed before we can process --color.

I see. I suspect so. I think this is not a good UI/UX; it seems at least the doc should be fixed. The current doc is this (cargo --help and man cargo respectively):

      --color <WHEN>            Coloring: auto, always, never

       --color when
           Control when colored output is used. Valid values:

           •  auto (default): Automatically detect if color support is available on
               the terminal.

           •  always: Always display colors.

           •  never: Never display colors.

           May also be specified with the term.color config value
           <https://doc.rust-lang.org/cargo/reference/config.html>.

I can't remember if I hacked in term.color / CARGO_TERM_COLOR support but that'd be the next thing id recommend.

Didn't work either. I'd be great if the doc would be fixed for the same reason described right above.

Otherwise, as an implementation detail, we do support NO_COLOR=1

Worked like a charm. Thank you.

$ NO_COLOR=1 cargo build --help

weihanglo · 2023-11-18T15:08:49Z

Thanks for calling them out. Some relevant contexts here about why it is hacky to support the option in CLI options and CARGO_TERM_COLOR:

That said, this is a thing I would like to see it fixed.

epage · 2023-11-18T15:40:19Z

@epage

Could you go into why you want to disable it?

Because personally I don't like the colors used.

Note that we are using terminal-configured colors, rather than something like RGB, so you do have full control over how this gets rendered in your terminal.

your-diary · 2023-11-18T15:53:38Z

@epage
Yes. But the setting is global; it cannot (or maybe difficult if not impossible) be set on a per-command basis.

Strictly speaking, the problem is not the color itself. I feel some inconsistency in how the color is used:

Actually, as you say, the color cyan has been used in Cargo. But, as far as I know, the main color is green and cyan is just temporarily displayed for highlighting purposes:

However, in the output of --help, cyan is used as one of the main colors:

Anyway, the essential problem is all of the Rust users with different personal preferences are now forced to be exposed to the colorful output.
cargo build --color never works. cargo build --help --color never does not work.

epage · 2023-11-19T01:22:19Z

Strictly speaking, the problem is not the color itself. I feel some inconsistency in how the color is used:

The reason I chose those colors for the help is that Green is used as more of a primary header while Cyan is a lower header / note.

cargo build --color never works. cargo build --help --color never does not work.

Usually a command-line flag is for transient behavior. If this is for user preference, a CLI flag wouldn't be appropriate.

As there are related issues for this., I'd recommend we shift the focus to the relevant ones as anything discussed here will be lost track of.

your-diary · 2023-11-19T02:04:54Z

@epage

Usually a command-line flag is for transient behavior. If this is for user preference, a CLI flag wouldn't be appropriate.

I don't think so. This is also a personal preference for design, I think.
I've learned 500 or more UNIX commands and many of them use CLI flags for user preferences.
And..., at least, cargo does support CLI flag for user preference:

function cargo() {
    if [[ $1 == 'build' ]]; then
        command cargo build --color never "$@"
    else
        command cargo "$@"
    fi
}

whether or not this is the way how --color is intended to be used.

As there are related issues for this., I'd recommend we shift the focus to the relevant ones as anything discussed here will be lost track of.

I do agree.
I just subscribed to #9012.
As #9012 is more essential, I will suspend my discussion and won't create a new issue.
For the time being, I'll use NO_COLOR=1.

Thank you for the series of the replies.

Pkgsrc changes: * Remove NetBSD-8 support (embedded LLVm requires newer C++ than what is in -8; it's conceivable that this could still build with an external LLVM) * undo powerpc 9.0 file naming tweak, since we no longer support -8. * Remove patch to LLVM for powerpc now included by upstream. * Minor adjustments, checksum changes etc. Upstream changes: Version 1.74.1 (2023-12-07) =========================== - [Resolved spurious STATUS_ACCESS_VIOLATIONs in LLVM] (rust-lang/rust#118464) - [Clarify guarantees for std::mem::discriminant] (rust-lang/rust#118006) - [Fix some subtyping-related regressions] (rust-lang/rust#116415) Version 1.74.0 (2023-11-16) ========================== Language -------- - [Codify that `std::mem::Discriminant<T>` does not depend on any lifetimes in T] (rust-lang/rust#104299) - [Replace `private_in_public` lint with `private_interfaces` and `private_bounds` per RFC 2145] (rust-lang/rust#113126) Read more in [RFC 2145](https://rust-lang.github.io/rfcs/2145-type-privacy.html). - [Allow explicit `#[repr(Rust)]`] (rust-lang/rust#114201) - [closure field capturing: don't depend on alignment of packed fields] (rust-lang/rust#115315) - [Enable MIR-based drop-tracking for `async` blocks] (rust-lang/rust#107421) Compiler -------- - [stabilize combining +bundle and +whole-archive link modifiers] (rust-lang/rust#113301) - [Stabilize `PATH` option for `--print KIND=PATH`] (rust-lang/rust#114183) - [Enable ASAN/LSAN/TSAN for `*-apple-ios-macabi`] (rust-lang/rust#115644) - [Promote loongarch64-unknown-none* to Tier 2] (rust-lang/rust#115368) - [Add `i686-pc-windows-gnullvm` as a tier 3 target] (rust-lang/rust#115687) Libraries --------- - [Implement `From<OwnedFd/Handle>` for ChildStdin/out/err] (rust-lang/rust#98704) - [Implement `From<{&,&mut} [T; N]>` for `Vec<T>` where `T: Clone`] (rust-lang/rust#111278) - [impl Step for IP addresses] (rust-lang/rust#113748) - [Implement `From<[T; N]>` for `Rc<[T]>` and `Arc<[T]>`] (rust-lang/rust#114041) - [`impl TryFrom<char> for u16`] (rust-lang/rust#114065) - [Stabilize `io_error_other` feature] (rust-lang/rust#115453) - [Stabilize the `Saturating` type] (rust-lang/rust#115477) - [Stabilize const_transmute_copy] (rust-lang/rust#115520) Stabilized APIs --------------- - [`core::num::Saturating`] (https://doc.rust-lang.org/stable/std/num/struct.Saturating.html) - [`impl From<io::Stdout> for std::process::Stdio`] (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStdout%3E-for-Stdio) - [`impl From<io::Stderr> for std::process::Stdio`] (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStderr%3E-for-Stdio) - [`impl From<OwnedHandle> for std::process::Child{Stdin, Stdout, Stderr}`] (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStderr%3E-for-Stdio) - [`impl From<OwnedFd> for std::process::Child{Stdin, Stdout, Stderr}`] (https://doc.rust-lang.org/stable/std/process/struct.Stdio.html#impl-From%3CStderr%3E-for-Stdio) - [`std::ffi::OsString::from_encoded_bytes_unchecked`] (https://doc.rust-lang.org/stable/std/ffi/struct.OsString.html#method.from_encoded_bytes_unchecked) - [`std::ffi::OsString::into_encoded_bytes`] (https://doc.rust-lang.org/stable/std/ffi/struct.OsString.html#method.into_encoded_bytes) - [`std::ffi::OsStr::from_encoded_bytes_unchecked`] (https://doc.rust-lang.org/stable/std/ffi/struct.OsStr.html#method.from_encoded_bytes_unchecked) - [`std::ffi::OsStr::as_encoded_bytes`] (https://doc.rust-lang.org/stable/std/ffi/struct.OsStr.html#method.as_encoded_bytes) - [`std::io::Error::other`] (https://doc.rust-lang.org/stable/std/io/struct.Error.html#method.other) - [`impl TryFrom<char> for u16`] (https://doc.rust-lang.org/stable/std/primitive.u16.html#impl-TryFrom%3Cchar%3E-for-u16) - [`impl<T: Clone, const N: usize> From<&[T; N]> for Vec<T>`] (https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#impl-From%3C%26%5BT;+N%5D%3E-for-Vec%3CT,+Global%3E) - [`impl<T: Clone, const N: usize> From<&mut [T; N]> for Vec<T>`] (https://doc.rust-lang.org/stable/std/vec/struct.Vec.html#impl-From%3C%26mut+%5BT;+N%5D%3E-for-Vec%3CT,+Global%3E) - [`impl<T, const N: usize> From<[T; N]> for Arc<[T]>`] (https://doc.rust-lang.org/stable/std/sync/struct.Arc.html#impl-From%3C%5BT;+N%5D%3E-for-Arc%3C%5BT%5D,+Global%3E) - [`impl<T, const N: usize> From<[T; N]> for Rc<[T]>`] (https://doc.rust-lang.org/stable/std/rc/struct.Rc.html#impl-From%3C%5BT;+N%5D%3E-for-Rc%3C%5BT%5D,+Global%3E) These APIs are now stable in const contexts: - [`core::mem::transmute_copy`] (https://doc.rust-lang.org/beta/std/mem/fn.transmute_copy.html) - [`str::is_ascii`] (https://doc.rust-lang.org/beta/std/primitive.str.html#method.is_ascii) - [`[u8]::is_ascii`] (https://doc.rust-lang.org/beta/std/primitive.slice.html#method.is_ascii) Cargo ----- - [fix: Set MSRV for internal packages] (rust-lang/cargo#12381) - [config: merge lists in precedence order] (rust-lang/cargo#12515) - [fix(update): Clarify meaning of --aggressive as --recursive] (rust-lang/cargo#12544) - [fix(update): Make `-p` more convenient by being positional] (rust-lang/cargo#12545) - [feat(help): Add styling to help output ] (rust-lang/cargo#12578) - [feat(pkgid): Allow incomplete versions when unambigious] (rust-lang/cargo#12614) - [feat: stabilize credential-process and registry-auth] (rust-lang/cargo#12649) - [feat(cli): Add '-n' to dry-run] (rust-lang/cargo#12660) - [Add support for `target.'cfg(..)'.linker`] (rust-lang/cargo#12535) - [Stabilize `--keep-going`] (rust-lang/cargo#12568) - [feat: Stabilize lints] (rust-lang/cargo#12648) Rustdoc ------- - [Add warning block support in rustdoc] (rust-lang/rust#106561) - [Accept additional user-defined syntax classes in fenced code blocks] (rust-lang/rust#110800) - [rustdoc-search: add support for type parameters] (rust-lang/rust#112725) - [rustdoc: show inner enum and struct in type definition for concrete type] (rust-lang/rust#114855) Compatibility Notes ------------------- - [Raise minimum supported Apple OS versions] (rust-lang/rust#104385) - [make Cell::swap panic if the Cells partially overlap] (rust-lang/rust#114795) - [Reject invalid crate names in `--extern`] (rust-lang/rust#116001) - [Don't resolve generic impls that may be shadowed by dyn built-in impls] (rust-lang/rust#114941) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. None this cycle.

rustbot assigned weihanglo Aug 28, 2023

rustbot added A-cli Area: Command-line interface, option parsing, etc. Command-run S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 28, 2023

epage force-pushed the help branch 2 times, most recently from 3d0d622 to 0fb78ce Compare August 29, 2023 13:42

epage mentioned this pull request Aug 29, 2023

fix(help): Explain --explain #12592

Merged

epage mentioned this pull request Aug 29, 2023

fix(help): Provide better commands heading for styling #12593

Merged

epage added a commit to epage/cargo that referenced this pull request Aug 29, 2023

fix(help): Remove redundant information from new/init

ad0a113

Auditing all of the `--help` in prep for rust-lang#12578 and noticed that we list the VCS information twice, once on our end and once by clap.

epage mentioned this pull request Aug 29, 2023

fix(help): Remove redundant information from new/init #12594

Merged

epage force-pushed the help branch from 0fb78ce to a460100 Compare August 30, 2023 20:57

rustbot added Command-add Command-bench Command-build Command-check Command-clean labels Aug 30, 2023

epage mentioned this pull request Sep 11, 2023

feat: Expose cargo's new styling crate-ci/clap-cargo#50

Merged

This was referenced Sep 11, 2023

feat(cargo): Style help like cargo nightly rust-lang/rustfmt#5908

Merged

Match nightly cargo's help output for cargo-clippy rust-lang/rust-clippy#11482

Closed

blyxyas mentioned this pull request Sep 13, 2023

Add colored help to be consistent with Cargo rust-lang/rust-clippy#11495

Merged

weihanglo mentioned this pull request Sep 15, 2023

Update cargo rust-lang/rust#115878

Merged

rfcbot added finished-final-comment-period FCP complete to-announce and removed final-comment-period FCP — a period for last comments before action is taken labels Sep 18, 2023

epage mentioned this pull request Sep 20, 2023

some way to disable bold colors? #2290

Open

epage mentioned this pull request Sep 26, 2023

Cargo and rustc are using different tones of red for errors on the terminal #12740

Open

ehuss added this to the 1.74.0 milestone Sep 30, 2023

epage mentioned this pull request Oct 24, 2023

Quest issue: how can we improve --help output #6104

Closed

weihanglo mentioned this pull request Nov 7, 2023

Cargo help colorful, missing in action. Rust 1.73 everything revealed mentions cargo help being colorful #12931

Closed

cnpryer mentioned this pull request Nov 9, 2023

Improving Terminal implementation cnpryer/huak#809

Open

weihanglo mentioned this pull request Nov 17, 2023

feat(cli): add color output for cargo --list #12992

Merged

4 tasks

weihanglo mentioned this pull request Nov 27, 2023

--help doesn't respect --color never nor CARGO_TERM_COLOR=never #13052

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(help): Add styling to help output #12578

feat(help): Add styling to help output #12578

epage commented Aug 28, 2023 •

edited

Loading

rustbot commented Aug 28, 2023

epage commented Aug 28, 2023

bors commented Aug 30, 2023

your-diary commented Nov 18, 2023

epage commented Nov 18, 2023

your-diary commented Nov 18, 2023

weihanglo commented Nov 18, 2023

epage commented Nov 18, 2023

your-diary commented Nov 18, 2023

epage commented Nov 19, 2023

your-diary commented Nov 19, 2023

feat(help): Add styling to help output #12578

feat(help): Add styling to help output #12578

Conversation

epage commented Aug 28, 2023 • edited Loading

What does this PR try to resolve?

How should we test and review this PR?

Additional information

rustbot commented Aug 28, 2023

epage commented Aug 28, 2023

bors commented Aug 30, 2023

your-diary commented Nov 18, 2023

epage commented Nov 18, 2023

your-diary commented Nov 18, 2023

weihanglo commented Nov 18, 2023

epage commented Nov 18, 2023

your-diary commented Nov 18, 2023

epage commented Nov 19, 2023

your-diary commented Nov 19, 2023

epage commented Aug 28, 2023 •

edited

Loading